feat: partial-success batch insert + exact import-retry idempotency (#3172, #3173)#3191
Merged
Merged
Conversation
… fires once (#3172) Root fix for the #3152 leftover: a batch that died in validation forced bulkWrite's whole-batch degradation, re-running beforeInsert hooks on the good rows (at-least-once side effects). - engine.insertMany(object, rows, options): new partial-success entry. Bad rows are culled per-row AFTER beforeInsert (encryption/validation/ autonumber failures each captured per row), survivors are written in one driver batch, and the return is one outcome per input row in input order ({ok,record} | {ok:false,error}). afterInsert, summary recompute and realtime fire only for written rows; dead rows consume no autonumber. Plain insert(array) semantics are untouched. - bulkWrite gains an optional writeBatchPartial: per-row failures from it are final verdicts (no writeOne degradation — that would re-fire the hooks); only a THROWN error (transient, mismatch) still degrades. - seed-loader's flushPendingInserts switches to writeBatchPartial and feature-detects engine.insertMany (legacy engines keep the old whole-array + degradation behavior). - protocol.insertManyData: partial-success sibling of createManyData (same readonly ingress strip), for the import runner. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
…adopt partial-success create (#3173, #3172) Pure-insert imports (no matchFields) had no natural key to recheck on a commit-then-lost-response retry, so they stayed at-least-once — a retry duplicated the whole batch (#3149 left this documented, #3173 tracked closing it). Every buffered CREATE row now gets a client-generated id at flush time (stable across attempts; an explicit user-supplied id is respected — both the SQL driver and bulkCreate honor caller ids). The retry path rechecks by id ($in) and re-inserts only rows that truly did not land. This is exact for every write mode: legitimate duplicate rows in insert mode resolve correctly because each copy has its own id — the case a natural-key or fingerprint recheck cannot distinguish. The old matchFields-based recheck is replaced by the id recheck. The import runner also prefers the protocol's new insertManyData (framework#3172) via bulkWrite's writeBatchPartial: a row that fails validation is a per-row verdict from one call, so a bad row no longer forces the whole-batch degradation that re-ran beforeInsert hooks on its siblings. Protocols without insertManyData keep the createManyData path. Flips the previously pinned at-least-once test: pure-insert retry after commit now expects zero duplicates. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📓 Docs Drift CheckThis PR changes 5 package(s): 39 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
os-zhuang
force-pushed
the
claude/evaluate-issues-3147-5152-1naxe6
branch
from
July 18, 2026 07:41
0147d3c to
c2367ab
Compare
…#3147–#3152, #3172, #3173) The 2026-07-06 incident's root lesson: the in-memory mock driver masked the bug (stored real booleans, never threw a transient error), so faithful local repro stayed green while turso dropped rows. Every probe so far uses a mock — this wires the REAL ObjectQL engine to the REAL SqlDriver (better-sqlite3, on-disk) and injects turso's actual failure shapes through a thin driver proxy: - #3172 insertMany culls a bad row per-row and writes survivors with a contiguous persistent autonumber sequence. - #3147 a transient blip on the parent summary UPDATE is retried; the roll-up recomputes correctly over a real SQL aggregate. - #3149/#3173 a bulkCreate that commits then loses its response (the exact turso shape the mock can't produce) does NOT duplicate seed rows — the natural-key recheck runs against the real table. - #3150 a transient blip on the self-referencing sequential seed path is retried, not dropped. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
os-zhuang
force-pushed
the
claude/evaluate-issues-3147-5152-1naxe6
branch
from
July 18, 2026 07:48
c2367ab to
e25808d
Compare
os-zhuang
marked this pull request as ready for review
July 18, 2026 08:05
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Follow-ups to the #3161 bulk-write hardening, closing the two gaps it left documented. Two commits, one per issue.
Fixes #3172, #3173.
#3172 — partial-success batch insert (root fix for the #3152 leftover)
A batch that died in validation forced
bulkWrite's whole-batch degradation, which re-ran the entireengine.insert— firing beforeInsert hooks a second time on the good rows.engine.insertMany(object, rows, options)(new, objectql): bad rows are culled per-row after beforeInsert (encryption / validation / autonumber failures each captured per row), survivors are written in one driver batch, and the return is one outcome per input row in input order ({ok, record}/{ok: false, error}). afterInsert, summary recompute, and realtime fire only for written rows; dead rows consume no autonumber sequence value. Plaininsert(array)semantics are untouched (still all-or-abort), so no existing caller changes behavior. Implemented as a flag inside the existing batch path — no code duplication.bulkWrite.writeBatchPartial(optional, core): per-row failures from it are final verdicts — nowriteOnedegradation for them (that would re-fire the hooks). Only a thrown error (transient,ERR_BULK_RESULT_MISMATCH) still degrades, with the same retry semantics aswriteBatch.flushPendingInsertsand the import runner (via the protocol's newinsertManyData, same readonly ingress strip ascreateManyData) switch over with feature detection — a legacy engine/protocol keeps the previous whole-array + degradation behavior.SummaryRecomputeError(bug(objectql): 汇总重算失败仅 warn——子记录批量插入报成功,父 summary 静默过期且无重试 #3147) composes: in partial mode itswrittenpayload is the outcome array, and the existing seed/import recovery handles it unchanged.Acceptance (pinned by tests): the #3152 probe
[{good1},{no-name},{good2}]now runs beforeInsert exactly once per row, afterInsert only for written rows, autonumbers stay contiguous.#3173 — exact import-retry idempotency via pre-assigned row ids
Pure-insert imports (no
matchFields) had no natural key to recheck on a commit-then-lost-response retry, so #3161 left them documented as at-least-once (a retry duplicated the batch).create/bulkCreatehonor caller-provided ids, and the protocol's readonly strip does not touchid, so this needs no schema migration.attempt > 1(or after an uncertain batch outcome) the runner rechecks by id$inand re-inserts only rows that truly did not land. This replaces the previousmatchFieldsapproximation and is exact for every write mode — including files with legitimate duplicate rows in insert mode, which no natural-key or fingerprint recheck could distinguish (each copy has its own id).sys_import_job_idcolumn, job-side accounting,created_at-window fingerprinting) were evaluated and rejected: the column needs a migration; fingerprinting has a real data-loss edge for back-to-back identical imports plus DB-vs-app clock skew on Postgres/MySQL.Tests
bulk-write.test.ts:writeBatchPartialverdict/degradation/mismatch cases (24 total in file).engine-insert-many.test.ts: the bug(objectql): 批降级重写时 beforeInsert 钩子对好行执行两次,autonumber 双消耗 #3152 probe (hooks once), single-driver-batch + contiguous autonumbers, all-bad batch, plain-insert regression.seed-loader-retry.test.ts: partial-success engine → bad row is per-row verdict, no re-run.import-runner-idempotency.test.ts: pure-insert exactly-once, legitimate-duplicate-rows survival,insertManyDatapartial path, plus existing bug(objectql): 汇总重算失败仅 warn——子记录批量插入报成功,父 summary 静默过期且无重试 #3147/bug(core/bulkWrite): 批量重试无幂等——驱动已提交但响应丢失时,重试导致整批重复插入 #3149 regressions.Verification
pnpm build— 71/71 (type-checks across all packages).pnpm test --concurrency=2— 131/131 task suites pass. (Unbounded concurrency OOM-kills random vitest workers in this container; the failures reproduced on untouched packages and each passed standalone.)🤖 Generated with Claude Code
https://claude.ai/code/session_01APnSDJneEDRh3Z51KGCLga
Generated by Claude Code